Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added normalize column to export grade book to remote gradebook #209

Conversation

amir-qayyum-khan
Copy link

Background

This PR resolves #147, resolves #187 and resolves #188

Fixed cherry pic issues of #201

What is done in this PR

Studio Updates: None
LMS Updates:

  • Added checkbox on legacy dashboard to indicate grades are normalize or not
  • On grade book export added column normalize with value 1 if gadebook is normalize else 0.

@pdpinch @pwilkins @noisecapella

  • screen shot 2016-02-10 at 5 58 49 pm
  • screen shot 2016-02-10 at 6 00 57 pm

@amir-qayyum-khan
Copy link
Author

@noisecapella fixed cherry pic issues as you suggested and made this PR.

@noisecapella
Copy link

Each cherry picked commit should contain exactly the diff that's present in the original commit so that we can deal with a clean git history. I'm going to go through and comment on each part of each diff that is out of place so that it can be fixed.

@amir-qayyum-khan amir-qayyum-khan force-pushed the enhancement/aq/two_way_grading_legacy_gradebook_mitocw#147 branch 2 times, most recently from f46fdb3 to ed5a805 Compare March 1, 2016 10:52
pwilkins and others added 5 commits March 1, 2016 16:19
Raw scores and max points are more useful to instructors in the
remote gradebook than the calculated scores sent now.

Fixes #36

@pdpinch
This will put the actual score and actual max score in scope for
the the return_csv function, so actual scores can be dumped.

The ultimate goal is to provide this data in the CSV dump that is
passed to Stellar via pylmod.

This is PR openedx#10395 on edX, and issue 95 on mitocw's edx fork.

https://github.com/edx/edx-platform/pull/10395

#95
…scores).

The progress page did a number of things that make performance terrible for
courses with large numbers of problems, particularly if those problems are
customresponse CapaModule problems that need to be executed via codejail.

The grading code takes pains to not instantiate student state and execute the
problem code. If a student has answered the question, the max score is stored
in StudentModule. However, if the student hasn't attempted the question yet, we
have to run the problem code just to call .max_score() on it. This is necessary
in grade() if the student has answered other problems in the assignment (so we
can know what to divide by). This is always necessary to know in
progress_summary() because we list out every problem there. Code execution can
be especially slow if the problems need to invoke codejail.

To address this, we create a MaxScoresCache that will cache the max raw score
possible for every problem. We select the cache keys so that it will
automatically become invalidated when a new version of the course is published.

The fundamental assumption here is that a problem cannot have two different
max score values for two unscored students. A problem *can* score two students
differently such that they have different max scores. So Carlos can have 2/3 on
a problem, while Lyla gets 3/4. But if neither Carlos nor Lyla has ever
interacted with the problem (i.e. they're just seeing it on their progress
page), they must both see 0/4 -- it cannot be the case that Carlos sees 0/3 and
Lyla sees 0/4.

We used to load all student state into two separate FieldDataCache instances,
after which we do a bunch of individual queries for scored items. Part of this
split-up was done because of locking problems, but I think we might have gotten
overzealous with our manual transaction hammer.

In this commit, we consolidate all state access in grade() and progress()
to use one shared FieldDataCache. We also use a filter so that we only pull
back StudentModule state for things that might possibly affect the grade --
items that either have scores or have children.

Because some older XModules do work in their __init__() methods (like Video),
instantiating them takes time, particularly on large courses. This commit also
changes the code that fetches the grading_context to filter out children that
can't possibly affect the grade.

Finally, we introduce a ScoresClient that also tries to fetch score
information all at once, instead of in separate queries. Technically, we are
fetching this information redundantly, but that's because the state and score
interfaces are being teased apart as we move forward. Still, this only
amounts to one extra SQL query, and has very little impact on performance
overall.

Much thanks to @adampalay -- his hackathon work in openedx#7168 formed the basis of
this.

https://openedx.atlassian.net/browse/CSM-17
@amir-qayyum-khan amir-qayyum-khan force-pushed the enhancement/aq/two_way_grading_legacy_gradebook_mitocw#147 branch from 1de0e9b to 01a6b0d Compare March 1, 2016 11:22
This change allows graded assignments to be added to a campus LMS
regardless of the granularity at which the problem sits. Previously
a grade could only be returned if the usage ID for the problem itself
was specified in the LTI launch.

The code assumes that courses taking advantage of this functionality
are arranged in a hiearchy (with sections being parents to verticals,
and verticals being parents to problems). When a grading event occurs
it traverses the parent hiearchy to identify any previous graded LTI
launches for which the new scoring event should generate a grade
update. It then calculates and sends scores to each of those outcome
services.

Since grade calculation is an expensive operation, the code optimizes
the case where a problem has been added only once as a leaf unit. In
that case it is able to behave as before, just taking the grade from
the signal without having to calculate grades for the whole course.
@amir-qayyum-khan
Copy link
Author

Fixed @noisecapella

@pdpinch
Copy link
Member

pdpinch commented Mar 3, 2016

Can you tell me why we need 0c49f2c ?

@amir-qayyum-khan
Copy link
Author

@pdpinch it has ProgressSummary code which i am using to get max point in legacy grade book ce17048#diff-5dd41ae768fed10fe37df53a1d9e78c8R779

@bdero bdero closed this Mar 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants